NOTICKET (feat): Add performance optimization for snowflake external tables#339
Open
Squisher23 wants to merge 4 commits into
Open
Conversation
…tables by limiting the partitions based on ldts
…ything in default
tkiehn
requested changes
Jul 13, 2026
tkiehn
left a comment
tkiehn
left a comment
Collaborator
There was a problem hiding this comment.
Hi @Squisher23 and thanks for reaching out!
Sorry for the late reply, I have reviewed the changes and in general the idea seems like a nice addition to the project.
I pointed out some suggested changes, if you are willing to implement these I would be happy to re-review the PR!
Best Regards
Theo
| {# Setting the column name for load date timestamp and record source to the alias coming from the attributes #} | ||
| {%- set ldts_alias = var('datavault4dbt.ldts_alias', 'ldts') -%} | ||
| {%- set rsrc_alias = var('datavault4dbt.rsrc_alias', 'rsrc') -%} | ||
| {%- set max_days_for_late_arriving_data = var('datavault4dbt.max_days_for_late_arriving_data', 'rsrc') -%} |
Collaborator
There was a problem hiding this comment.
Default value is set as 'rsrc', to keep the behaviour backwards compatible, as in no filter is applied, it should default to 0.
Comment on lines
+207
to
+210
| {% if max_days_for_late_arriving_data|int != 0 %} | ||
|
|
||
| AND {{ ldts }} >= DATEADD(day, {{ max_days_for_late_arriving_data }}*-1, CURRENT_DATE) | ||
| {%- endif -%} |
Collaborator
There was a problem hiding this comment.
Suggested change
| {% if max_days_for_late_arriving_data|int != 0 %} | |
| AND {{ ldts }} >= DATEADD(day, {{ max_days_for_late_arriving_data }}*-1, CURRENT_DATE) | |
| {%- endif -%} | |
| {% if max_days_for_late_arriving_data|int != 0 %} | |
| AND {{ ldts }} >= DATEADD(day, {{ max_days_for_late_arriving_data }}*-1, CURRENT_DATE) | |
| {%- endif -%} |
Small readability adjustment
|
|
||
| #Stage Configuration | ||
| datavault4dbt.copy_rsrc_ldts_input_columns: false | ||
| datavault4dbt.max_days_for_late_arriving_data: |
Collaborator
There was a problem hiding this comment.
Suggested change
| datavault4dbt.max_days_for_late_arriving_data: | |
| datavault4dbt.max_days_for_late_arriving_data: 0 |
Same as above, the default should be set in a way that does not change existing behaviour
tkiehn
added
the
waiting-on-response
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add performance optimization for snowflake external tables tables by limiting the partitions based on ldts
Description
Snowflake has some weaknesses in external table implementation and only does partition pruning when usings literals as filter on the partition ( see (https://community.snowflake.com/s/article/Pruning-is-not-happening-subquery for details)
The issue #335 already solved our most critical issue with the runtime of satelites, but also links, hubs and tracking-satelites have been getting slower and slower (from seconds to several minutes) with the increasing number of parquet-files in our data lake.
This PR tries to solve the problem by adding a new parameter datavault4dbt.max_days_for_late_arriving_data and filtering all staging tables to the last x days that were configured. Adding this feature speed up our dbt from 30 Minutes to just 5 minutes and should not have other consequences as long as you run dbt more often than the max_days_for_late_arriving_data and you don't have any data sources that deliver data later than the max_days_for_late_arriving_data-Parameter. To recreate the whole vault the parameter needs to be deleted or set to a value high enough
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.
Test Configuration:
Checklist: